Add an iterator to `Distribution` #361

pitdicker · 2018-03-31T16:48:48Z

This tries to solve the same problem as #275, having a nice way to sample from distributions using an iterator. See also #58.

I have added a sample_iter method to Distribution for now. But I don't mind if it is a method on Rng instead, or on both...

With benchmarks similar to those in #275, the results are all over the place. In all situations it is slightly slower than iter::repeat, but usually not by more than a couple of percent. The exception is Uniform, where we use that values generated by the RNG directly:

test gen_1k_fill                   ... bench:       1,129 ns/iter (+/- 1) = 906 MB/s
test gen_1k_gen_iter               ... bench:         729 ns/iter (+/- 1) = 1404 MB/s
test gen_1k_iter_repeat            ... bench:         372 ns/iter (+/- 0) = 2752 MB/s
test gen_1k_sample_iter            ... bench:         626 ns/iter (+/- 1) = 1635 MB/s <- new

Example of using Distribution::sample_iter instead of Rng::gen_iter:

use distributions::{Alphanumeric, Range, Uniform};
let rng = thread_rng();

// Vec of 16 x f32:
// old
let v: Vec<f32> = iter::repeat(()).map(|()| rng.gen()).take(16).collect();
// new
let v: Vec<f32> = Uniform.sample_iter(&mut rng).take(16).collect();

// String
// old
let s: String = rng.gen_ascii_chars().take(10).collect();
// new
let s: String = Alphanumeric.sample_iter(&mut rng).take(7).collect();

// Combined values
// old
println!("{:?}", rng.gen_iter::<(f64, bool)>().take(5)
                    .collect::<Vec<(f64, bool)>>());
// new
println!("{:?}", Uniform.sample_iter(&mut rng).take(5)
                        .collect::<Vec<(f64, bool)>>());

// Dice-rolling:
// old
let die_range = Range::new_inclusive(1, 6);
let mut roll_die = iter::repeat(()).map(|()| rng.sample(die_range));
while roll_die.next().unwrap() != 6 {
    println!("Not a 6; rolling again!");
}
// new
let die_range = Range::new_inclusive(1, 6);
let mut roll_die = die_range.sample_iter(&mut rng);
while roll_die.next().unwrap() != 6 {
    println!("Not a 6; rolling again!");
}

pitdicker · 2018-04-01T11:51:15Z

I also added sample_iter to Rng, and it is slightly nicer to use in most cases:

use rand::{thread_rng, Rng};
use rand::distributions::{Alphanumeric, Range, Uniform};

let mut rng = thread_rng();

// Vec of 16 x f32:
let v: Vec<f32> = thread_rng().sample_iter(&Uniform).take(16).collect();

// String:
let s: String = rng.sample_iter(&Alphanumeric).take(7).collect();

// Combined values
println!("{:?}", thread_rng().sample_iter(&Uniform).take(5)
                             .collect::<Vec<(f64, bool)>>());

// Dice-rolling:
let die_range = Range::new_inclusive(1, 6);
let mut roll_die = rng.sample_iter(&die_range);
while roll_die.next().unwrap() != 6 {
    println!("Not a 6; rolling again!");
}

dhardy

Looking nice — perhaps it is worth having both methods!

dhardy · 2018-04-01T16:20:09Z

src/lib.rs

+    /// let mut rng = thread_rng();
+    ///
+    /// // Vec of 16 x f32:
+    /// let v: Vec<f32> = thread_rng().sample_iter(&Uniform).take(16).collect();


Nice, but lets not promote bad styles in examples by creating a local rng handle then not using it.

I wanted to show that it was also possible to use thread_rng() directly. But yes, bad style. Will change.

I think most people will figure that out.

dhardy · 2018-04-01T16:23:08Z

src/lib.rs

+    /// }
+    /// ```
+    fn sample_iter<'a, T, D: Distribution<T>>(&'a mut self, distr: &'a D)
+        -> distributions::DistIter<'a, D, Self, T> where Self: Sized


Interesting that this takes the distribution by reference while sample takes it by value. I'm not saying this is wrong. If we have both, then distr.iter(&mut rng) is still an option when passing by reference is required, so this could take the distribution by value (which would consume die_range above but only if the & is removed — we also implement Distribution for the references).

I am not sure we can have both, because the iterator expects an reference instead of something owned. Or do we want to use some sort of Cow or MaybeOwned trick?

I think it will work if you just change to distr: D here; essentially Distribution::iter receives a reference from this function stack.

error[E0597]: `distr` does not live long enough --> src/lib.rs:414:9 | 414 | distr.sample_iter(self) | ^^^^^ borrowed value does not live long enough 415 | } | - borrowed value only lives until here | note: borrowed value must be valid for the lifetime 'a as defined on the method body at 411:5... --> src/lib.rs:411:5 | 411 | / fn sample_iter<'a, T, D: Distribution<T>>(&'a mut self, distr: D) 412 | | -> distributions::DistIter<'a, D, Self, T> where Self: Sized 413 | | { 414 | | distr.sample_iter(self) 415 | | } | |_____^ error: aborting due to previous error For more information about this error, try `rustc --explain E0597`.

Sorry, of course the iterator is returned from sample_iter!

pitdicker · 2018-04-01T17:27:22Z

Do you want to do the honours of merging today (and I the rebasing...)?

Add an iterator to `Distribution`

pitdicker added 2 commits March 31, 2018 19:01

Add Distribution::sample_iter

425da90

Add iterator benchmarks

b9a2c59

pitdicker force-pushed the sample_iter branch from 6f6cb19 to b9a2c59 Compare March 31, 2018 17:01

Also add sample_iter to Rng

6682c5b

pitdicker force-pushed the sample_iter branch from 0ae7841 to 6682c5b Compare April 1, 2018 11:48

dhardy reviewed Apr 1, 2018

View reviewed changes

dhardy merged commit 8c099c3 into rust-random:master Apr 1, 2018

pitdicker deleted the sample_iter branch April 1, 2018 17:28

pitdicker pushed a commit that referenced this pull request Apr 4, 2018

Merge pull request #361 from pitdicker/sample_iter

dba18b8

Add an iterator to `Distribution`

pitdicker mentioned this pull request May 15, 2018

Remove Distribution::sample_iter #455

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add an iterator to `Distribution` #361

Add an iterator to `Distribution` #361

pitdicker commented Mar 31, 2018

pitdicker commented Apr 1, 2018

dhardy left a comment

dhardy Apr 1, 2018

pitdicker Apr 1, 2018

dhardy Apr 1, 2018

dhardy Apr 1, 2018

pitdicker Apr 1, 2018

dhardy Apr 1, 2018

pitdicker Apr 1, 2018

dhardy Apr 1, 2018

pitdicker commented Apr 1, 2018

Add an iterator to Distribution #361

Add an iterator to Distribution #361

Conversation

pitdicker commented Mar 31, 2018

pitdicker commented Apr 1, 2018

dhardy left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pitdicker commented Apr 1, 2018

Add an iterator to `Distribution` #361

Add an iterator to `Distribution` #361